Unnecessary requirement for project in resources#280
Open
bhouse-nexthop wants to merge 11 commits intoapache:mainfrom
Open
Unnecessary requirement for project in resources#280bhouse-nexthop wants to merge 11 commits intoapache:mainfrom
project in resources#280bhouse-nexthop wants to merge 11 commits intoapache:mainfrom
Conversation
…it from ip address Support both explicit project specification and automatic inheritance from IP address: - Port forward inherits project from ip_address_id when no explicit project is set - Uses projectid=-1 for universal IP address lookup across projects - Updates state with inherited or explicit project during read operations - Schema updated to mark project as Optional and Computed - Maintains backward compatibility with existing implementations Changes: - Added Computed: true to project field - Modified resourceCloudStackPortForwardCreate to fetch IP address and inherit project - Modified resourceCloudStackPortForwardRead to handle universal project search - Added TestAccCloudStackPortForward_projectInheritance test case - Updated documentation with inheritance behavior and example All 4 port forward acceptance tests passing.
When creating a network in a VPC, if no explicit project is specified, the network will now automatically inherit the project from the VPC. This simplifies configuration by eliminating the need to specify the project parameter when creating networks in VPCs that belong to projects. Changes: - Modified resourceCloudStackNetworkCreate to fetch VPC details and inherit projectid when vpc_id is provided but project is not - Modified resourceCloudStackNetworkRead to handle networks with inherited projects by trying projectid=-1 when no explicit project is set - Added TestAccCloudStackNetwork_vpcProjectInheritance test case - Updated documentation with inheritance behavior and example All 14 network acceptance tests pass.
…network Implement automatic project inheritance for cloudstack_instance resource: - Instance inherits project from network_id when no explicit project is set - Only applies to Advanced networking zones - Uses projectid=-1 for universal network lookup across projects - Updates state with inherited project during read operations Changes: - Modified resourceCloudStackInstanceCreate to fetch network and inherit project - Modified resourceCloudStackInstanceRead to set project in state - Added TestAccCloudStackInstance_networkProjectInheritance test case - Updated documentation with inheritance behavior and example All 12 instance acceptance tests passing.
… VPC or network Implement automatic project inheritance for cloudstack_ipaddress resource: - IP address inherits project from vpc_id when no explicit project is set - IP address inherits project from network_id when no explicit project is set - Uses projectid=-1 for universal VPC/network lookup across projects - Updates state with inherited project during read operations Changes: - Modified resourceCloudStackIPAddressCreate to fetch VPC/network and inherit project - Modified resourceCloudStackIPAddressRead to handle universal project search - Added TestAccCloudStackIPAddress_vpcProjectInheritance test case - Added TestAccCloudStackIPAddress_networkProjectInheritance test case - Updated documentation with inheritance behavior and examples All 5 IP address acceptance tests passing.
…om VPC Implement automatic project inheritance for cloudstack_network_acl resource: - Network ACL inherits project from vpc_id when no explicit project is set - Uses projectid=-1 for universal VPC lookup across projects - Updates state with inherited project during read operations - Schema updated to mark project as Computed Changes: - Modified resourceCloudStackNetworkACLCreate to fetch VPC and inherit project - Modified resourceCloudStackNetworkACLRead to handle universal project search - Updated schema to mark project as Computed: true - Added TestAccCloudStackNetworkACL_vpcProjectInheritance test case - Updated documentation with inheritance behavior and example All 5 network ACL acceptance tests passing.
bhouse-nexthop
force-pushed
the
resolve-project
branch
from
7dd4118 to
a55fe76
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the project attribute optional across several CloudStack resources by inheriting the project context from related resources (VPC, network, or IP address), aligning provider behavior with CloudStack API requirements and addressing #278.
Changes:
- Implement project inheritance logic in
port_forward,network,network_acl,ipaddress, andinstanceresources (plus read-path fallbacks for inherited project state). - Mark
projectasComputedwhere needed to prevent perpetual diffs when the provider populates state. - Add acceptance coverage and update docs to describe automatic project inheritance patterns.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/r/port_forward.html.markdown | Adds inheritance-focused examples and clarifies project inheritance behavior. |
| website/docs/r/network_acl.html.markdown | Adds inheritance example and updates project argument description. |
| website/docs/r/network.html.markdown | Documents VPC network project inheritance and updates project argument description. |
| website/docs/r/ipaddress.html.markdown | Adds examples for VPC/network usage and documents project inheritance. |
| website/docs/r/instance.html.markdown | Adds example demonstrating instance project inheritance from network. |
| cloudstack/resource_cloudstack_port_forward.go | Inherits project from IP address; adjusts read/list scoping to avoid missing rules in project context. |
| cloudstack/resource_cloudstack_network_acl.go | Inherits project from VPC and persists it in state for subsequent reads. |
| cloudstack/resource_cloudstack_network.go | Inherits projectid from VPC during create; adds read fallback to locate inherited-project networks. |
| cloudstack/resource_cloudstack_ipaddress.go | Inherits projectid from VPC/network during create. |
| cloudstack/resource_cloudstack_instance.go | Inherits projectid from network during create; adds read fallback to locate inherited-project instances. |
| cloudstack/resource_cloudstack_port_forward_test.go | Adds acceptance test for port forward project inheritance. |
| cloudstack/resource_cloudstack_network_test.go | Adds acceptance test for network project inheritance from VPC. |
| cloudstack/resource_cloudstack_network_acl_test.go | Adds acceptance test for network ACL project inheritance from VPC. |
| cloudstack/resource_cloudstack_ipaddress_test.go | Adds acceptance tests for IP address project inheritance from VPC/network. |
| cloudstack/resource_cloudstack_instance_test.go | Adds acceptance test for instance project inheritance from network. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
|
@bhouse-nexthop can you check Copilot's comments? I have started the acceptance tests for this PR. |
In resourceCloudStackInstanceRead, setting project before calling
setValueOrID(d, "project", ...) breaks the "preserve ID vs name"
behavior: if the user configured project as an ID, this d.Set overwrites
it with the project name, causing setValueOrID to treat it as a name and
persist the name instead of the ID.
This commit removes the early d.Set("project", vm.Project) and relies on
the existing setValueOrID(d, "project", vm.Project, vm.Projectid) later
in the function to properly preserve whether the user specified an ID or
name.
Changed 'deploy this instance to' to 'deploy this resource to' in the project argument description to accurately reflect that this is the Network ACL resource, not an instance resource.
Changed 'deploy this instance to' to 'deploy this network to' in the project argument description to accurately reflect that this is the Network resource, not an instance resource.
Changed 'deploy this instance to' to 'deploy this IP address to' in the project argument description to accurately reflect that this is the IP address resource, not an instance resource.
Added cloudstack_instance.web resource definition to the project inheritance example so it can be applied as-is. The example previously referenced cloudstack_instance.web.id without defining the resource.
The testAccCheckCloudStackNetworkACLProjectInherited function doesn't actually validate project inheritance - it only checks the ACL name. Since the test already has a state assertion that checks the project attribute (resource.TestCheckResourceAttr), this helper function is redundant and misleading. Removed the function and its call from the test.
Collaborator
Author
done, can you re-request copilot review to validate? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
projectcan be learned/inherited based on the other parameters specified. The cloudstack provider was mandatingprojectin instances where the actual cloudstack api does not require, such as on cloudstack_port_forward.This adds proper inheritance making the project completely optional in a lot of cases. You can't create an network in a different project than its associated vpc, likewise you can't create a network acl in a different project than the vpc, so you shouldn't have to specify it.
In some instances like cloudstack_port_forward and cloudstack_network_acl the project probably should be removed completely from the schema as it serves zero actual purpose, but I chose to preserve compatibility with existing users instead.
Fixes #278